Skip to content

Add nginxGateway.name usage in case of its presence instead of nameOverride #3519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Add nginxGateway.name usage in case of its presence instead of nameOverride #3519

wants to merge 1 commit into from

Conversation

@tataruty
Copy link
Contributor

@tataruty tataruty commented Jun 16, 2025 •
edited
Loading

Add nginxGateway.name usage in case of its presence instead of nameOverride

Proposed changes

As a user of NGF's Helm Charts
I want to use a nginxGateway.name to configure the names of my deployments
So that I don't have to use nameOverride to override the entire name.

Testing:

name = momo, nameOverride = overrrmi, releaseName = chart,  chart.name = nginx-gateway-fabric
result: chart-momo
proxy: momo-proxy-config
config: momo-config


name = chart, nameOverride = overrrmi, releaseName = chart, chart.name = nginx-gateway-fabric
result: chart
proxy: chart-proxy-config
config: chart-config


name =  , nameOverride = chart, releaseName = chart, chart.name = nginx-gateway-fabric
result: chart
proxy: chart-proxy-config
config: chart-config

name = momo, nameOverride = , releaseName = chart, chart.name = nginx-gateway-fabric
result: chart-momo
proxy: momo-proxy-config
config: momo-config

name = , nameOverride = overrrmi, releaseName = chart, chart.name = nginx-gateway-fabric
result: chart-overrrmi
proxy: overrrmi-proxy-config
config: overrrmi-config

name = , nameOverride = , releaseName = chart, chart.name = nginx-gateway-fabric
result: chart-nginx-gateway-fabric
proxy: chart-proxy-config
config: chart-config


name = , nameOverride = , releaseName = nginx-gateway-fabric, chart.name = nginx-gateway-fabric
result: nginx-gateway-fabric
proxy: nginx-gateway-fabric-proxy-config
config: nginx-gateway-fabric-config

 name = , nameOverride = nginx-gateway-fabric, releaseName = chart, chart.name = nginx-gateway-fabric
result: chart-nginx-gateway-fabric
proxy: nginx-gateway-fabric-proxy-config
config: nginx-gateway-fabric-config

name = nginx-gateway-fabric, nameOverride = , releaseName = chart, chart.name = nginx-gateway-fabric
result: chart-nginx-gateway-fabric
proxy: nginx-gateway-fabric-proxy-config
config: nginx-gateway-fabric-config

Closes #2490

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

N/A

@tataruty tataruty requested a review from a team as a code owner June 16, 2025 15:28
@nginx-bot
Copy link

nginx-bot bot commented Jun 16, 2025

Hi @tataruty! Welcome to the project! 🎉

Thanks for opening this pull request!
Be sure to check out our Contributing Guidelines while you wait for someone on the team to review this.

@nginx-bot nginx-bot bot added the community label Jun 16, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 16, 2025 •
edited
Loading

✅ All required contributors have signed the F5 CLA for this PR. Thank you!
Posted by the CLA Assistant Lite bot.

@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request tests Pull requests that update tests helm-chart Relates to helm chart labels Jun 16, 2025
@tataruty
Copy link
Contributor Author

I have hereby read the F5 CLA and agree to its terms

I have hereby read the F5 CLA and agree to its terms

@tataruty tataruty force-pushed the feature/add_name_to_helm_chart branch from 19eb65f to e516510 Compare June 17, 2025 14:01
@tataruty
Copy link
Contributor Author

I have hereby read the F5 CLA and agree to its terms

@sjberman
Copy link
Collaborator

@tataruty In general, as an NGF team member, you'll want to create your PRs from a branch off of the main repository, instead of a fork. In the past we exclusively used forks, however we had to update our workflows in order to be able to run NGINX Plus tests, since those are internal access only.

Community contributors still use forks, and we can use forks for simple changes, like docs and such. But in general we want to be able to run the NGINX Plus tests for any product, chart, or test changes, so in order to do that we have to create branches off of the main repo.

sjberman
sjberman reviewed Jun 17, 2025
View reviewed changes
charts/nginx-gateway-fabric/values.yaml
kind: deployment

# -- The name of the NGINX Gateway Fabric deployment - if not present, then by default uses release name given during installation.
name: release name
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we just leave this field as an empty string. Then the comment here is more accurate, otherwise the default value is technically release name in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you! that was the one confusing thing!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess i will recreate PR for main repo without a branch, need to be added to github team too

@tataruty tataruty closed this Jun 18, 2025
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in NGINX Gateway Fabric Jun 18, 2025
@tataruty
Copy link
Contributor Author

closed since opened not from the fork: #3527

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sjberman sjberman sjberman left review comments

Assignees

No one assigned

Labels

community documentation Improvements or additions to documentation enhancement New feature or request helm-chart Relates to helm chart tests Pull requests that update tests

Projects

NGINX Gateway Fabric
Status: ✅ Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add nginxGateway.name in helm chart

2 participants

@tataruty @sjberman